Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Replaces camelcase with package-local SVG basename conversion helpers to reduce dependencies.
Changes:
- Adds identical
toPascalCasehelpers to both Jest SVG transforms. - Removes direct
camelcasedependencies and updates the lockfile. - Direct regression coverage for the helpers is still needed.
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
packages/fxa-react/svg-transform.js |
Adds local PascalCase conversion. |
packages/fxa-react/package.json |
Removes camelcase. |
packages/fxa-settings/config/jest/fileTransform.js |
Adds local PascalCase conversion. |
packages/fxa-settings/package.json |
Removes camelcase. |
yarn.lock |
Updates workspace dependency records. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
## Because - The `toPascalCase` helper that replaced `camelcase` has no direct test in either copy. - Coverage today is indirect. `LoadingSpinner` and `ButtonIcon` render an SVG and never assert the generated name, so the separator, camelCase and digit rules can break without a test failing. ## This pull request - Adds `packages/fxa-react/lib/svg-transform.test.ts`. It asserts the generated component name for six basename shapes. - Adds `packages/fxa-settings/src/lib/jest-file-transform.test.ts` with the same six shapes, against the second copy of the helper. - Touches no helper logic. This is a test-only follow-up to review comments on the original change. ## Issue that this pull request solves Closes: https://mozilla-hub.atlassian.net/browse/FXA-13929
vbudhram
changed the title
toufali
approved these changes
Aug 18, 2026
toufali
left a comment
toufali
left a comment
Member
There was a problem hiding this comment.
The helper is copy-pasted into both transforms. Leaving it instead of unifying since there's no clean shared place to put it. If a third copy shows up, DRY it then!
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Because
toPascalCasehelper that replacedcamelcasehas no direct test in either copy.LoadingSpinnerandButtonIconrender an SVG and never assert the generated name, so the separator, camelCase and digit rules can break without a test failing.This pull request
packages/fxa-react/lib/svg-transform.test.ts. It asserts the generated component name for six basename shapes.packages/fxa-settings/src/lib/jest-file-transform.test.tswith the same six shapes, against the second copy of the helper.Issue that this pull request solves
Closes: https://mozilla-hub.atlassian.net/browse/FXA-13929
Checklist
Put an
xin the boxes that applyHow to review (Optional)
it.eachtable.fxa-reactspec first. Thefxa-settingsspec is the same table against the other copy.fxa-settingssets jestrootstosrc, so a spec placed besideconfig/jest/fileTransform.jswould never run. The spec sits undersrc/lib/and imports the transform by relative path. No jest config changed. I also broke one expectation in each file on purpose, confirmed both specs failed, then restored them.Screenshots (Optional)
Other information (Optional)
The six cases are
trash-icon,trash_icon,trash.icon,closeIcon,icon2xandclose. They pin the three separators, an input that is already camelCase, the digit-to-letter uppercase rule, and a plain single word.Both transforms prefix
Svg, so both specs assert the fullSvgTrashIconshape.The helper is still duplicated across the two packages. That is deliberate for now. Consolidating it would touch package boundaries and jest configs, which is a bigger call than this follow-up. These two specs are what stop the copies drifting apart.